Jehan [Fri, 17 Feb 2023 22:27:32 +0000 (23:27 +0100)]
babl: limit LUT creation for 4→8 and 4→16 bpp further.
Looking at LUT conversion code, we are clearly only supporting
converting from "u8" (to "u16" or "float" respectively).
Conversions such as "YA half" to "R'G'B'A float" for instance would be
completely broken (it was trying to use only one byte of the "YA half",
assumed it was "u8" and transform it to "float"). This is why the sample
image in #85 was appearing on display as more transparent than it should
when converted to 16-bit floating point grayscale image.
Jehan [Fri, 17 Feb 2023 21:45:11 +0000 (22:45 +0100)]
babl: make LUT from 2bpp to 4bpp totally generic.
The deleting of the first byte when creating the LUT, then copying the
first byte from source to destination, worked for cases such as "YA u8" to
"RGBA u8" but it would not work for say:
* "Y half" to anything: in such a case, deleting the first byte in LUT's
content, when creating it, we are basically deleting source's color
information!
* "YA u8" to "Y float": in such a case, when copying the first byte, at
conversion time, we are destroying the result color (which didn't need
any copying of alpha channel as there is no alpha channel in the
result).
* And so on.
In such cases, it's just better to include everything in the LUT, in a
totally model-agnostic way. In some cases, the alpha channel will be
stored in the LUT, in others, not. But it's not really a problem. It
doesn't even make the LUT bigger; if anything, it makes the LUT creation
and the conversion calculus simpler, hence faster.
Øyvind Kolås [Tue, 14 Feb 2023 22:13:33 +0000 (23:13 +0100)]
babl: narrow conditions for doing LUT conversion
We can do LUTs where the LUT depends only on the 24first bit - for some
formats this ends up including half of the alpha, which is OK when
3->4bpp conversions copy the last 8bits.
Jehan [Tue, 14 Feb 2023 09:50:20 +0000 (10:50 +0100)]
Issue #85: Troubles opening 16-bit floating point grayscale files.
All current LUT conversion and creation clearly assume that we are
operating on 1 byte per channel (the flag we use right now to determine
if we may have a LUT is even called `is_u8_color_conv`). So let's
enforce this.
Otherwise we might end up with problems such as in issue #85, where we
convert from or to "YA u16", which is still under 4bpp but not 1bpc. As
a consequence, the first byte of the alpha only was copied and the rest
(including the second alpha byte) was used as belonging to the color
components.
Of course, we can update this code later to also support such cases
(taking into account the source and target alpha size), but for now,
let's simply exclude these.
Jehan [Mon, 13 Feb 2023 22:34:48 +0000 (23:34 +0100)]
babl: fix LUT from grayscale to RGBA images.
Images from "Y'A" to "R'G'B'A" for instance would end up completely
transparent because we were not reporting the alpha channel from the
source to destination buffer.
Jehan [Sun, 12 Feb 2023 12:05:14 +0000 (13:05 +0100)]
babl: don't create LUT for destination formats with premultiplied colors.
This is a different issue than previous commit, but was revealed by
fixing the bug. So in a way, it's a followup.
When the destination format uses premultiplied colors, we would end up
with a LUT where all colors in the table are (0, 0, 0) since we store
them with alpha = 0. We could also generate them with alpha = 1, but
then we'd have to special-case the conversion step, re-multiplying each
pre-multiplied value with the real alpha, etc. So instead, I just bypass
these conversion cases and don't create LUT for these.
For the record, we had this case on GIMP when rendering u8 images to
display, since it would require for instance a LUT from R'G'B'A u8 to
cairo-ARGB32 (Cairo's ARGB32 being pre-multiplied with alpha). So the
whole image would render black (even though, when color-picking colors,
we'd see pixels' colors are not black).
Note that this bug existed before, but was simply hidden by the bug
fixed in the previous commit (since conversion was done twice: first
with LUT, then through usual conversion paths, the second conversion was
hiding the broken LUT conversion).
Jehan [Sun, 12 Feb 2023 11:58:19 +0000 (12:58 +0100)]
Issue #84: do not process color conversion twice.
Until now babl_fish_lut_process_maybe() was always returning 0, even
when a LUT existed, hence a LUT-based conversion happened.
This was bad, first because it was inefficient (the point of the LUT is
that we didn't have to process through usual conversion code paths). But
worse: when the source and destination buffers were the same, we would
end up getting wrong result (since we'd have the source converted
in-place, then re-converted, hence double conversion!).
This was the reason for issue #84 (see screenshot showing very wrong
colors because of double conversion).
On Windows, the wrappers internally convert from UTF8 to UTF16 and
call the wide char routines, that way the string need not be limited
to the ANSI character set. Outside of Windows the wrappers are no-op.
babl_dir_list is implemented using opendir / readdir on non-Windows
and _wfindfirst64 / _wfindnext64 on Windows.
One of GIMP layer modes was using wrongfully these coefficients on
non-linear data (see gimp#3471). It was not clear from babl docs whether
babl was returning wrong coefficients for non-linear spaces, or whether
we were supposed to only ever use these coefficients on linear data.
This is the second case. So let's make the in-code docs clearer.
Øyvind Kolås [Sat, 12 Nov 2022 22:36:09 +0000 (23:36 +0100)]
babl: add babl_gc
The LUT garbage collection was running ad-hoc as a side effect of processing,
this is problematic since it could trigger during processing of other threads
that might be using or freeing the same fish leading to races. The new babl_gc
function is to be called from the main thread, at times when processing is not
occuring, calling babl_gc is not neccesary in short lived commandline tools
and similar.
This will output the result as int or float values (depending on output
format), separated by space.
It would allow to use the output of this script for testing conversion
(with expected output), and can also be used for chaining calls, making
this a re-entrant CLI tool.
- Add a --help/-h option.
- Detect invalid options (argument starting with '-').
- Support '--' option to indicate end of options. This can be
particularly useful to set negative component values in floating point
formats.
- Print usage in various error types (when they are of wrong
syntax/option types), additionally to printing it when --help is
called.
Fix alignment of vector of floats in babl-rgb-converter.c
Downstream Bug (Gentoo): https://bugs.gentoo.org/857708
Bug: https://gitlab.gnome.org/GNOME/babl/-/issues/76
Thanks-to: Alexander Bezrukov <phmagic@mail.ru> Signed-off-by: Alexander Bezrukov <phmagic@mail.ru> Signed-off-by: Sergey Torokhov <torokhov-s-a@yandex.ru>
- Add missing lcms dependencies. That's needed when lcms is a subproject
otherwise those targets does not find its headers.
- Add lcms2 wrap so meson can build it as subproject in case the
dependency is not found on system.
- Fix couple meson warnings
- Use meson.override_dependency() so babl can be used as subproject
without hardcoding "babl_dep" variable name in main project.
Eli Schwartz [Mon, 14 Mar 2022 00:26:05 +0000 (20:26 -0400)]
meson: fix misspelled kwarg name
set10 doesn't have a `Description` kwarg, it does have a `description`
kwarg though.
This caused the conf variable to not have a description when it should
have one, and newer versions of Meson with better argument validation
error out with:
Øyvind Kolås [Wed, 23 Feb 2022 16:34:59 +0000 (17:34 +0100)]
ci: disable to save the environment
Arch linux continue to be a bit incompatible with the buildfarm,
CI is already bad for the environment, cpu consuming expected
failures are even worse.
The CI might turn up again backed by debian, but it is better to
turn it off for now.
Andrzej Hunt [Sat, 4 Sep 2021 11:43:29 +0000 (13:43 +0200)]
babl_format_new: also free doc to plug leak
doc and name are allocated via babl_strdup, hence if we don't use them
we need to free both of them - therefore add a free(doc) to match the
existing free(name).
format_new (called just below) transfers ownership of doc, hence it's
correct not to free doc in the case where we're instantiating a new
format - whereas name is copied within format_new, and therefore needs to be
free'd in either scenario.
Leak seen while running gimp+babl built with ASAN:
Direct leak of 99 byte(s) in 1 object(s) allocated from:
#0 0x5e87cd in malloc /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
#1 0x7f038605e2b8 in babl_malloc /home/ahunt/git/babl/_build/../babl/babl-memory.c:115:9
#2 0x7f038605eec7 in babl_strdup /home/ahunt/git/babl/_build/../babl/babl-memory.c:296:9
#3 0x7f038605110f in babl_format_new /home/ahunt/git/babl/_build/../babl/babl-format.c:376:17
#4 0x7f037313a432 in init /home/ahunt/git/babl/_build/../extensions/cairo.c:569:25
#5 0x7f03860412a1 in babl_extension_load /home/ahunt/git/babl/_build/../babl/babl-extension.c:226:7
#6 0x7f0386040d1c in babl_extension_load_dir /home/ahunt/git/babl/_build/../babl/babl-extension.c:268:19
#7 0x7f03860409a8 in babl_extension_load_dir_list /home/ahunt/git/babl/_build/../babl/babl-extension.c:338:17
#8 0x7f038607d6cd in babl_init /home/ahunt/git/babl/_build/../babl/babl.c:154:7
#9 0x7f0386bce5ab in gegl_post_parse_hook /home/ahunt/git/gegl/_build/../gegl/gegl-init.c:542:3
#10 0x7f03844aad8a in g_option_context_parse /home/ahunt/git/glib/_build/../glib/goption.c:2228:12
#11 0x7f0386bce381 in gegl_init /home/ahunt/git/gegl/_build/../gegl/gegl-init.c:189:8
#12 0xef1f6b in gimp_init_for_testing /home/ahunt/git/gimp/app/tests.c:69:3
[...]
SUMMARY: AddressSanitizer: 99 byte(s) leaked in 1 allocation(s).
Øyvind Kolås [Wed, 9 Feb 2022 19:03:46 +0000 (20:03 +0100)]
babl: fix mutex lock/unlocks symmetry in babl_fish()
In the case where the fish we are to look up is the memcpy fish we were
not locking the mutex. Whereas the common cleanup after the branches
assumes the lock is held. This is probably the cause of gimp#7632.